Upload API Prep Part 1: Add db-migrate, extract config.ts, change tsoa entrypoint and use tsx in prod #33

francisduvivier · 2024-11-27T19:08:01Z

So this PR does some project setup:

Set up path aliases
For more readable and copy pastable imports
Auto Lint populate.ts
By prettier
Rename populate function to populateDB
Name was not clear for me
Change tsoa entryfile to app.ts and extract config.ts
Centralized config into a file and change the tsoa entry point because otherwise the entrypoint was already importing tsoa generated code.
Caused the following commit after: Fix vite unit tests
Improve logging in case of tsoa validation error
So we see more easily what went wrong when a request is not valid.
Disable write api when not in development mode
This is needed since we do not have authentication yet on the write api
Add db-migrate package with wiring, rationale and instructions
So this adds a system for database migrations.
Rebuild package-lock.json since the arm64 version of esbuild is not in there (@esbuild/darwin-arm64).
Just a package.lock update
Fix production setup
The path aliases and imports without extensions were giving issues after typescript compilation, leading to a situation where devleopment works fine but production crashes.
So I chose here to just run the same way in prod as in development, namely using tsx. The alternative was doing bundling with esbuild for prod.
But I don't see an issue or big disadvantage for us with using tsx in prod and the advantage is simplicity and debuggability and least chance of discrepancies between development and prod.
Implement using pg_try_advisory_lock for db-migrations to be able to run mulpile instances again
So we now always do db-migrate at the startup of the program. This way, we are sure that the database in the program is compatible with the queries in the program. However, these migrations are not necessarily safe to run in parallel, so in this commit I added a setup to ensure that the db-migrations are not run in parallel even when we start multiple instances of the program.

github-actions · 2024-11-27T19:18:18Z

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	21.43%	191 / 891
🔵	Statements	21.43%	191 / 891
🔵	Functions	40%	10 / 25
🔵	Branches	58.82%	10 / 17

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/app.ts	100%	100%	100%	100%
src/config.ts	100%	100%	100%	100%
src/disableWriteWhenNotDev.ts	0%	100%	100%	0%	2-18
src/index.ts	0%	0%	0%	0%	1-21
src/openapi.ts	100%	100%	100%	100%
src/populateDB.ts	0%	0%	0%	0%	1-353
src/db/connectionPool.ts	66.66%	100%	50%	66.66%	25-32
src/db/migrations.ts	0%	0%	0%	0%	1-31
src/util/logging.ts	0%	100%	100%	0%	2-16

Generated in workflow #84 for commit 0923e6c by the Vitest Coverage Report Action

src/__test__/populateDB.ts

src/populate.ts

src/populateDB.ts

paulinevos · 2024-12-04T08:48:36Z

Hey Francis, appreciate the elaborate per commit explanations in the PR description here. In future, may I suggest adding these explanations to the commit message itself? This makes it easy to review commit by commit, and will also help anyone who's looking through our history to figure out why a specific change was made.

src/__test__/populateDB.ts

paulinevos · 2024-12-04T08:52:27Z

src/util/logging.ts

+  app.use(
+    (err: unknown, _req: Request, _res: Response, next: NextFunction): void => {
+      if (err instanceof ValidateError) {
+        console.error(


May want to consider middleware like this that automatically adds the appropriate status code to the response actually. Nice.

Yeah that could be cool, haven't been busy with the status codes yet.

src/index.ts

paulinevos

Looks very good, just two questions to clarify

paulinevos · 2024-12-04T08:57:08Z

Dockerfile

-COPY package*.json ./
-RUN npm install
-COPY . .
-RUN npm run build


Wait why are we removing this?

There is no build anymore, we just run straight with tsx.
Here the commit and a bit about the rationale:

Fix production setup
The path aliases and imports without extensions were giving issues after typescript compilation, leading to a situation where devleopment works fine but production crashes.
So I chose here to just run the same way in prod as in development, namely using tsx. The alternative was doing bundling with esbuild for prod.
But I don't see an issue or big disadvantage for us with using tsx in prod and the advantage is simplicity and debuggability and least chance of discrepancies between development and prod.

package.json

francisduvivier · 2024-12-04T17:02:11Z

Hey Francis, appreciate the elaborate per commit explanations in the PR description here. In future, may I suggest adding these explanations to the commit message itself? This makes it easy to review commit by commit, and will also help anyone who's looking through our history to figure out why a specific change was made.

@paulinevos Good point, i'll rebase to add the extra info.
Edit: done now.

Name did not make it clear to me what this file was for

Centralized config into a file and change the tsoa entry point because otherwise the entrypoint was already importing tsoa generated code.

So this adds a system for database migrations.

…n there (@esbuild/darwin-arm64).

The path aliases and imports without extensions were giving issues after typescript compilation, leading to a situation where devleopment works fine but production crashes. So I chose here to just run the same way in prod as in development, namely using tsx. The alternative was doing bundling with esbuild for prod. But I don't see an issue or big disadvantage for us with using tsx in prod and the advantage is simplicity and debuggability and least chance of discrepancies between development and prod.

…run multiple instances again So we now always do db-migrate at the startup of the program. This way, we are sure that the database in the program is compatible with the queries in the program. However, these migrations are not necessarily safe to run in parallel, so in this commit I added a setup to ensure that the db-migrations are not run in parallel even when we start multiple instances of the program.

edwinm · 2024-12-04T20:01:36Z

Dockerfile

+
+COPY --chown=node:node ./public ./public
+COPY --chown=node:node ./src ./src
+
 USER node


I'm not sure, but if you put USER node at the top of the file, you might get rid of all the --chown=node:node?

Yes I copied this over from the original dockedfile here, but would indeed be interesting to check if this is actually needed.

Compose file names are now more consistent. Npm script is now shorter

Set up path aliases

75a17b4

francisduvivier requested review from edwinm, paulinevos, zera1ul and Park0 November 27, 2024 19:08

francisduvivier force-pushed the feature/26-upload-api-part-1 branch from 5128389 to 1ef7a09 Compare November 27, 2024 19:17

francisduvivier linked an issue Nov 27, 2024 that may be closed by this pull request

DB migration setup for future DB changes #31

Closed

github-advanced-security bot found potential problems Nov 27, 2024

View reviewed changes

src/__test__/populateDB.ts Fixed Show fixed Hide fixed

src/__test__/populateDB.ts Fixed Show fixed Hide fixed

Auto Lint populate.ts

d12d1bf

francisduvivier force-pushed the feature/26-upload-api-part-1 branch from 06ec830 to 529fec3 Compare November 27, 2024 19:33

francisduvivier changed the title ~~Add db migrate and change tsoa entrypoint~~ Add db-migrate, extract config.ts and change tsoa entrypoint Nov 27, 2024

francisduvivier force-pushed the feature/26-upload-api-part-1 branch from 529fec3 to 1408127 Compare November 27, 2024 19:54

francisduvivier changed the title ~~Add db-migrate, extract config.ts and change tsoa entrypoint~~ Upload API Prep Part 1: Add db-migrate, extract config.ts and change tsoa entrypoint Nov 27, 2024

francisduvivier mentioned this pull request Nov 27, 2024

26 db migration, type safety, domain layer and insert project api #32

Closed

github-advanced-security bot found potential problems Nov 27, 2024

View reviewed changes

src/populate.ts Fixed Show fixed Hide fixed

src/populate.ts Fixed Show fixed Hide fixed

francisduvivier force-pushed the feature/26-upload-api-part-1 branch from 48b84c9 to d84ea54 Compare November 27, 2024 23:32

github-advanced-security bot found potential problems Nov 27, 2024

View reviewed changes

src/populateDB.ts Dismissed Show dismissed Hide dismissed

src/populateDB.ts Dismissed Show dismissed Hide dismissed

francisduvivier force-pushed the feature/26-upload-api-part-1 branch from cfa87d8 to c1330de Compare December 3, 2024 19:03

francisduvivier changed the title ~~Upload API Prep Part 1: Add db-migrate, extract config.ts and change tsoa entrypoint~~ Upload API Prep Part 1: Add db-migrate, extract config.ts and change tsoa entrypoint and use tsx in prod Dec 4, 2024

francisduvivier changed the title ~~Upload API Prep Part 1: Add db-migrate, extract config.ts and change tsoa entrypoint and use tsx in prod~~ Upload API Prep Part 1: Add db-migrate, extract config.ts, change tsoa entrypoint and use tsx in prod Dec 4, 2024

paulinevos reviewed Dec 4, 2024

View reviewed changes

src/__test__/populateDB.ts Outdated Show resolved Hide resolved

paulinevos reviewed Dec 4, 2024

View reviewed changes

src/index.ts Show resolved Hide resolved

paulinevos reviewed Dec 4, 2024

View reviewed changes

Francis Duvivier added 3 commits December 4, 2024 18:05

Rename populate function to populateDB

976fa81

Name did not make it clear to me what this file was for

Change tsoa entryfile to app.ts and extract config.ts

32f566c

Centralized config into a file and change the tsoa entry point because otherwise the entrypoint was already importing tsoa generated code.

Improve logging in case of tsoa validation error

526e193

Francis Duvivier added 5 commits December 4, 2024 18:07

Disable write api when not in development mode

229ae05

Centralized config into a file and change the tsoa entry point because otherwise the entrypoint was already importing tsoa generated code.

Add db-migrate package with wiring, rationale and instructions

0c2779a

So this adds a system for database migrations.

Rebuild package-lock.json since the arm64 version of esbuild is not i…

9dbf70c

…n there (@esbuild/darwin-arm64).

francisduvivier force-pushed the feature/26-upload-api-part-1 branch from c1330de to a6af53e Compare December 4, 2024 17:18

francisduvivier requested a review from paulinevos December 4, 2024 18:05

edwinm reviewed Dec 4, 2024

View reviewed changes

edwinm approved these changes Dec 4, 2024

View reviewed changes

Rename docker compose file names and docker-compose-test: scripts

0923e6c

Compose file names are now more consistent. Npm script is now shorter

francisduvivier force-pushed the feature/26-upload-api-part-1 branch from b452048 to 0923e6c Compare December 5, 2024 17:37

paulinevos approved these changes Dec 6, 2024

View reviewed changes

francisduvivier merged commit fc54897 into main Dec 6, 2024
3 checks passed

francisduvivier deleted the feature/26-upload-api-part-1 branch December 9, 2024 16:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upload API Prep Part 1: Add db-migrate, extract config.ts, change tsoa entrypoint and use tsx in prod #33

Upload API Prep Part 1: Add db-migrate, extract config.ts, change tsoa entrypoint and use tsx in prod #33

francisduvivier commented Nov 27, 2024 •

edited

Loading

github-actions bot commented Nov 27, 2024 •

edited

Loading

paulinevos commented Dec 4, 2024

paulinevos Dec 4, 2024

francisduvivier Dec 4, 2024

paulinevos left a comment

paulinevos Dec 4, 2024

francisduvivier Dec 4, 2024 •

edited

Loading

francisduvivier commented Dec 4, 2024 •

edited

Loading

edwinm Dec 4, 2024

francisduvivier Dec 5, 2024 •

edited

Loading

Upload API Prep Part 1: Add db-migrate, extract config.ts, change tsoa entrypoint and use tsx in prod #33

Upload API Prep Part 1: Add db-migrate, extract config.ts, change tsoa entrypoint and use tsx in prod #33

Conversation

francisduvivier commented Nov 27, 2024 • edited Loading

github-actions bot commented Nov 27, 2024 • edited Loading

Coverage Report

paulinevos commented Dec 4, 2024

paulinevos Dec 4, 2024

Choose a reason for hiding this comment

francisduvivier Dec 4, 2024

Choose a reason for hiding this comment

paulinevos left a comment

Choose a reason for hiding this comment

paulinevos Dec 4, 2024

Choose a reason for hiding this comment

francisduvivier Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

francisduvivier commented Dec 4, 2024 • edited Loading

edwinm Dec 4, 2024

Choose a reason for hiding this comment

francisduvivier Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

francisduvivier commented Nov 27, 2024 •

edited

Loading

github-actions bot commented Nov 27, 2024 •

edited

Loading

francisduvivier Dec 4, 2024 •

edited

Loading

francisduvivier commented Dec 4, 2024 •

edited

Loading

francisduvivier Dec 5, 2024 •

edited

Loading